fix: accept IDs as well as names for --channel, --environment and --tenant - #702

The name fallback inside Tenants.GetByIdentifier (GetByName in the SDK) issues a single tenants?partialName=<name> query with no take and scans only the first page (server default page size) for an exact match - it never paginates. partialName is a contains filter, so a tenant whose exact name sorts after a page's worth of other tenants containing the same substring (e.g. --tenant Smith in a space with Aaron Smith, Bob Smith, ... Zoe Smith) resolves to ErrItemNotFound and the deploy fails with cannot find a tenant with the ID or name of 'Smith' - even though that exact name worked before this PR, when it was passed straight through and matched server-side.

Since this can turn a working --tenant <name> into a hard failure on the deploy hot path, consider resolving names with a paginated exact-match lookup (the way the old selectors.FindEnvironment walked GetNextPage) rather than relying on GetByIdentifier's first-page fallback. (Projects.GetByIdentifier has the same latent flaw for --project, but this PR extends the exposure to --tenant on release deploy and runbook run.)

Actioned in 93ab1c6.

Confirmed against the SDK we pin (go-octopusdeploy/v2 v2.114.1, pkg/tenants/tenant_service.go): GetByIdentifier (L234) falls back to GetByName (L213), which does a single s.Get(TenantsQuery{PartialName: name}) and then scans only tenants.Items — the first page — before returning services.ErrItemNotFound. No take, no GetNextPage. The finding holds exactly as described.

selectors.FindTenant no longer calls GetByIdentifier. It does the ID lookup itself (Tenants.GetByID, treating a 404 as "not an ID, try the name") and then walks every page of the partialName search looking for a case-insensitive exact match, keeping ID-beats-name precedence. The paging loop uses resultPage.GetNextPage(octopus.Tenants.GetClient()), which returns nil, nil when Links.PageNext is empty, so it terminates on the last page.

Covered by TestFindTenants/finds an exact name match beyond the first page of the partial name search in pkg/question/selectors/find_test.go, which serves Aaron Smith on page one with a PageNext link and the exact Smith on page two.

Residual, deliberately not fixed here: Projects.GetByIdentifier still has the same first-page-only name fallback, and selectors.FindProject still goes through it. That's pre-existing on main and this PR doesn't widen it, so I've left it — say the word if you want it pulled in.

PR #703 raises the same tenant-pagination finding; the conclusion there is the same, and the fix lives in selectors.FindTenant, which both branches share.

Because the fallback is all-or-nothing over the whole list, a list mixing a regular and an ephemeral environment can never resolve: the regular lookup errors on the ephemeral name, the ephemeral lookup then errors on the regular name, and the user sees cannot find an environment with the ID or name of '<ephemeral env>' - blaming an environment that exists. Before this PR the --no-prompt path passed the names through and left the mix to the server.

If mixed lists are meant to be invalid that's defensible, but the message is misleading. Resolving per-identifier (regular list first, then ephemeral; each list fetched at most once) would keep single-type lists behaving exactly as now, while letting mixed lists either resolve or fail with an accurate message.

Actioned in 3d8333b. The finding holds: the fallback was all-or-nothing over the whole list, so a mixed list could never resolve and the error named an environment that exists.

The resolver moved out of deploy.go into selectors.ResolveEnvironmentNames and now works per identifier: each one is matched against the regular environment list first, and the ephemeral list is consulted only for identifiers that list doesn't have — fetched once, lazily, so a single-type list makes exactly the same calls it did before. A genuine miss now names the identifier that actually went missing.

Two things that came out of the rework:

• it used to fall back on any error from the regular lookup, including a transport failure; now the regular lookup's error is returned as-is and only a miss triggers the ephemeral list.
• findEphemeralEnvironments indexed name-then-ID, so a name could shadow another environment's ID. It now indexes ID-first, matching the precedence everywhere else.

Covered by TestResolveEnvironmentNames in pkg/question/selectors/find_test.go: resolves a mix of regular and ephemeral environments, doesn't look at ephemeral environments when everything resolves (asserts the ephemeral endpoint is never hit), and names the environment that is actually missing.

Deliberate residual: if the ephemeral endpoint itself errors (it doesn't exist on every server version) the error is swallowed and reported as cannot find an environment with the ID or name of '<identifier>', because the identifier genuinely isn't a regular environment and a "v2 environments endpoint returned 404" message would be worse for the common case. That does mean a real transport failure against that endpoint is reported as not-found.

PR #703 raises the same mixed-list finding; same conclusion, and the fix is in the shared selectors resolver so both branches pick it up.

Question: release deploy got the ephemeral fallback (resolveEnvironmentNames) but the runbook path uses bare selectors.FindEnvironments, which only sees GET /environments/all - and ephemeral environments aren't in that list. Before this PR the value was passed straight through for the server to resolve, so if a runbook run can target an ephemeral environment by name, that now fails client-side with cannot find an environment with the ID or name of '...'.

If runbook runs can never target ephemeral environments this is fine as-is; otherwise this call probably wants the same fallback deploy got.

Answering the question: oversight, not intentional. Actioned in 3d8333b (and tightened in 956f445).

runbook run now calls the same selectors.ResolveEnvironmentNames(octopus, f.GetCurrentSpace(), ...) that release deploy's no-prompt path uses, so an environment name that used to be passed through for the server to resolve is no longer rejected client-side just because it isn't in GET /environments/all.

956f445 closes the other half of it: the prompt path used to re-resolve the canonical name through executionscommon.FindEnvironments when picking run targets, which would have failed for an ephemeral environment even after the fallback was in place. That path now reuses the ID the up-front resolve produced, so an ephemeral name survives the whole flow.

Unverified: whether the runbook-run executions API actually accepts an ephemeral environment by name server-side. I have no instance to try it against. What I can say is that the CLI no longer decides the answer for the server — the behaviour is back to "pass it on and let the server rule", which is what it was before this branch. If you know runbook runs can't target ephemeral environments at all, the right change is a specific error message rather than the generic cannot find an environment..., and I'd want that from you rather than guessing.

Keep the resolved environment identity for later runbook lookups.

This replaces the original identifier with a name, but AskDbRunbookRunQuestions / AskGitRunbookRunQuestions later pass that name through the ID-first executionscommon.FindEnvironments again when selecting run targets. The by-tag path does the same for its prompted-variable preview.

Using the collision already covered by the selector tests: environment A has ID Environments-99 and name Environments-13; environment B has ID Environments-13 and name production. Passing --environment Environments-99 first resolves correctly to A, then the later lookup resolves its name to B. I reproduced that exact two-stage lookup: the final ID is Environments-13 instead of Environments-99. The run still submits A's name, but target selection and prompted-variable checks use B's preview.

Please retain the resolved environment objects/IDs for those lookups, or defer conversion to names until building the execution request. Add a runbook-level collision test covering the target or by-tag preview, rather than only testing each resolver independently.

Actioned in 956f445. Reproduced your two-stage collision before fixing it, and the resolved identity is now carried through instead of the name being re-resolved.

What changed:

• selectors.ResolveEnvironments returns []*ResolvedEnvironment{ID, Name} — the ID and name of whichever environment each identifier picks out, regular or ephemeral. ResolveEnvironmentNames is now a thin wrapper over it for callers that only want names (release deploy's no-prompt path).
• runbookRun resolves once and threads the resolved slice through runDbRunbook / runGitRunbook / runRunbooksByTag and into AskDbRunbookRunQuestions / AskGitRunbookRunQuestions, the same way project is already threaded. Nothing on the command path resolves an environment twice.
• askRunbookTargets / askGitRunbookTargets now take environmentIDs []string. They only ever used env.ID, and taking IDs makes it impossible to hand them a name to re-resolve.
• the by-tag prompted-variable preview uses the resolved ID. It also used to call FindEnvironments once per matching runbook; it's now one lookup for the whole loop.
• the FindEnvironments name lookup survives only as a fallback for the exported Ask* entry points, which can legitimately be called without pre-resolved environments.

Test, as asked, at the runbook level rather than per-resolver: TestRunbookRunByTag_UsesTheResolvedEnvironmentForThePreview in pkg/cmd/runbook/run/run_test.go. It uses your collision (Environments-99/Environments-13 vs Environments-13/production), runs runbook run --runbook-tag nightly --environment Environments-99 in automation mode, and asserts the preview goes to .../runbookRuns/preview/Environments-99 and that /environments/all is fetched exactly once. I checked it fails on the old shape — reinstating the FindEnvironments call makes the mock server report /api/Spaces-1/environments/all where it expected the Environments-99 preview. There's also TestResolveEnvironments in find_test.go pinning the ID/name pair for the colliding identifier.

Two residuals worth naming:

• askRunbookPreviewVariables still receives the interactively-selected environments only, so it gets nothing when --environment came from the command line, and prompted variables aren't previewed on that path. That's pre-existing and unrelated to the double-resolution, so I left it rather than change interactive behaviour inside this PR. Happy to do it separately — it would mean users who pass --environment interactively start getting prompted-variable questions they don't get today, which is a behaviour change worth its own decision.
• executionscommon.AskTenantsAndTags likewise still gets the interactive selection only, for the same reason.

release deploy doesn't have the equivalent bug: deployRun never pre-converts options.Environments to names, and AskQuestions resolves them exactly once (deploy.go L492-L500), so there's no second lookup to collide.